feat: add reasoning token support to OpenRouter integration#3264
Conversation
Extract reasoning/thinking content from OpenRouter responses and attach to ChatMessage via ReasoningContent. Override run(), run_async(), and stream handler. Handle multi-turn by re-injecting reasoning_details into formatted message dicts. 13 new tests covering extraction, conversion, streaming, multi-turn, edge cases, and async paths. Closes deepset-ai#2181
|
|
|
Heads-up for maintainers This PR is from a fork and touches integrations whose integration tests require API keys. Affected integrations:
Please run the integration tests locally ( |
github-actions
Bot
added
the
type:documentation
Coverage report (openrouter)
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
Add tests for empty messages, logprobs, malformed tool call JSON, empty streaming choices, async empty messages, and async streaming with reasoning content.
logprobs Choice requires top_logprobs field, was missing. Inline imports in test methods trigger PLC0415 lint error.
|
Brought up to date |
|
@julian-risch could you take a look at this PR when you get the time please? |
- Accept `list[ChatMessage] | str` in run/run_async and normalize via `_normalize_messages`. Fixes the mypy override (LSP) error and restores the string-input support that OpenAIChatGenerator now provides. - Stop overriding the streaming handlers and the chunk converter; inherit them from the parent and instead warn when streaming is combined with reasoning, since reasoning content can only be reconstructed from non-streaming responses (mirrors MistralChatGenerator). This removes ~150 lines of duplicated parent code that also silently dropped `reasoning_details` during streaming, so multi-turn reasoning pass-back no longer no-ops when streaming. - Restore the explanatory comments that were dropped from `to_dict` and `_prepare_api_call` (the `openai_endpoint` hint mechanism and the `to_dict` override rationale). - Bump the `haystack-ai` dependency to >=2.30.0 for `_normalize_messages`. - Tests: add a live `test_live_run_with_reasoning` integration test (verified against deepseek/deepseek-r1) plus string-input and streaming-with-reasoning-warning unit tests; drop the tests for the removed streaming converter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
julian-risch
force-pushed
the
openrouter-reasoning-support
branch
from
6c2627b to
5664c24
Compare
julian-risch
removed
the
type:documentation
julian-risch
left a comment
julian-risch
left a comment
There was a problem hiding this comment.
Thank you for opening this PR @ArkaD171717! It's ready to be merged now. Congratulations on your first contribution to Haystack!
Before merging I pushed a commit with a few adjustments:
-
Fixed CI failures: run/run_async now accept list[ChatMessage] | str and call _normalize_messages. This was a recent change in haystack after you opened the PR. I therefore bumped the dependency to haystack-ai>=2.30.0 too.
-
Streaming reasoning: removed the overridden stream handlers/chunk converter (which silently dropped reasoning_details during streaming). streaming now inherits the parent handlers and warns when combined with reasoning, capturing reasoning for non-streaming only. This mirrors MistralChatGenerator and cuts ~150 lines of duplicated parent code. This consistency with MistralChatGenerator makes it easier for us to maintain the code base.
-
Restored the explanatory comments dropped from to_dict/_prepare_api_call. Please keep changes to unrelated comments in any future PRs as limited as possible.
-
Added a live
test_live_run_with_reasoningintegration test because we shouldn't rely only on unit tests for this new feature. I can confirm that the integration test passed on my local machine.
|
Thank you! Edits look good to me. |
3 participants
Related Issues
Proposed Changes:
OpenRouter returns reasoning/thinking content as extra fields on the completion response (
reasoning/reasoning_details), which the parentOpenAIChatGeneratordoesn't know about, so they were silently dropped. This adds reasoning support toOpenRouterChatGenerator:run()/run_async()to convert completions with a dedicated_convert_openrouter_completion_to_chat_message, which reads the reasoning fields viagetattr(OpenRouter attaches them as extra attributes on standard OpenAI SDK models, not typed fields) and attaches them toChatMessageasReasoningContent.reasoninggeneration kwarg, the component logs a warning pointing the user to non-streaming mode. This mirrorsMistralChatGenerator._prepare_api_callre-injectsreasoning_detailsinto the formatted message dicts before sending them back to the API, sinceto_openai_dict_format()strips reasoning.run()/run_async()now acceptlist[ChatMessage] | strand normalize via_normalize_messages, matching the parent. Requireshaystack-ai>=2.30.0.How did you test it?
test_live_run_with_reasoningintegration test, verified againstdeepseek/deepseek-r1.Notes for the reviewer
reasoning. This keeps the integration aligned withMistralChatGeneratorand avoids duplicating the parent's streaming code.Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.